Opt in to remaining Rails 7.1 defaults #30332
Conversation
| @@ -57,9 +57,7 @@ | |||
| module Mastodon | |||
| class Application < Rails::Application | |||
| # Initialize configuration defaults for originally generated Rails version. | |||
| config.load_defaults 7.0 | |||
|
|
|||
| config.active_record.marshalling_format_version = 7.1 | |||
There was a problem hiding this comment.
This was previously set here to work around a config bug where it didn't get set correctly from the new_framework_defaults file. With bump to 7.1 defaults, it isn't needed.
| # in 7.0), then you need to configure SHA-256 for Active Record Encryption: | ||
| # Rails.application.config.active_record.encryption.hash_digest_class = OpenSSL::Digest::SHA256 | ||
| # | ||
| # If you don't currently have data encrypted with Active Record encryption, you can disable this setting to |
There was a problem hiding this comment.
I think we are effectively in this scenario -- we have the recently enabled otp AR encryption column, but that has only existed on 7.1 and has never been in a release, so I think we are in the SHA256 case described above, and having the framework default of not backwards-supporting SHA1 is acceptable.
There was a problem hiding this comment.
I would have thought that as well, but:
May 31 14:59:06 mastodon bundle[993665]: E, [2024-05-31T14:59:06.438057 #993665] ERROR -- : [64fdf096-f096-476a-a51b-9cc20e91919a]
May 31 14:59:06 mastodon bundle[993665]: [64fdf096-f096-476a-a51b-9cc20e91919a] ActiveRecord::Encryption::Errors::Decryption (ActiveRecord::Encryption::Errors::Decryption):
May 31 14:59:06 mastodon bundle[993665]: [64fdf096-f096-476a-a51b-9cc20e91919a]
May 31 14:59:06 mastodon bundle[993665]: [64fdf096-f096-476a-a51b-9cc20e91919a] app/controllers/concerns/auth/two_factor_authentication_concern.rb:32:in `valid_otp_attempt?'
May 31 14:59:06 mastodon bundle[993665]: [64fdf096-f096-476a-a51b-9cc20e91919a] app/controllers/concerns/auth/two_factor_authentication_concern.rb:74:in `authenticate_with_two_factor_via_otp'
May 31 14:59:06 mastodon bundle[993665]: [64fdf096-f096-476a-a51b-9cc20e91919a] app/controllers/concerns/auth/two_factor_authentication_concern.rb:51:in `authenticate_with_two_factor'
May 31 14:59:06 mastodon bundle[993665]: [64fdf096-f096-476a-a51b-9cc20e91919a] lib/mastodon/rack_middleware.rb:9:in `call'
I'm not completely sure this is because of this particular setting, though.
| # replicas will not be able to deserialize `BigDecimal` arguments from this | ||
| # serializer. Therefore, this setting should only be enabled after all replicas | ||
| # have been successfully upgraded to Rails 7.1. | ||
| # Rails.application.config.active_job.use_big_decimal_serializer = true |
There was a problem hiding this comment.
This one needs consideration as to what the impact would be of enabling now -vs- waiting for a proper 4.3 release running Rails 7.1 first (and then enable this in a future release).
| # | ||
| # In Rails 7.1, the new default is `:json_allow_marshal` which serializes and | ||
| # deserializes with `ActiveSupport::JSON`, but can fall back to deserializing | ||
| # with `Marshal` so that legacy messages can still be read. |
There was a problem hiding this comment.
I think this fallback behavior helps us.
| # | ||
| # If you are performing a rolling deploy of a Rails 7.1 upgrade, wherein servers | ||
| # that have not yet been upgraded must be able to read messages from upgraded | ||
| # servers, first deploy without changing the serializer, then set the serializer |
There was a problem hiding this comment.
This is another one where we may want a 4.3 release running rails 7.1 first, and then make this change after.
| # not yet been upgraded must be able to read messages from upgraded servers, | ||
| # leave this optimization off on the first deploy, then enable it on a | ||
| # subsequent deploy. | ||
| # Rails.application.config.active_support.use_message_serializer_for_metadata = true |
There was a problem hiding this comment.
Another one - maybe should wait for post-7.1 release.
mjankowski
force-pushed
the
rails-7-1-defaults-enable
branch
from
57ed14c
to
dacd7e4
Compare
mjankowski
force-pushed
the
rails-7-1-defaults-enable
branch
from
dacd7e4
to
9ec35cf
Compare
I believe that the remainder of the options which were not already enabled here (in the file deleted in this PR) are safe to enable at this point, and we can flip over to the 7.1 defaults.
However - I know we also want to be deliberate with how people step through upgrades, and we may want the 4.3 release to include an update to Rails 7.1 (4.2.x is on Rails 7.0) but NOT immediately enable all these defaults.
Vaguely related previous PRs:
bin/rails app:updatewith Rails 7.1 #27522Opening this to step through the final remaining changes in this diff, and coordinate the interaction of release timing/versions with when to make this change.